-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[flang][OpenMP] Use OmpDirectiveSpecification in REQUIRES #160595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…sing The DECLARE_VARIANT directive takes two names separated by a colon as an argument: base-name:variant-name. Define OmpBaseVariantNames to represent this, since no existing argument alternative matches it. However, there is an issue. The syntax "name1:name2" can be the argument to DECLARE_VARIANT (if both names are OmpObjects), but it can also be a reduction-specifier if "name2" is a type. This conflict can only be resolved once we know what the names are, which is after name resolution has visited them. The problem is that name resolution has side-effects that may be (practically) impossible to undo (e.g. creating new symbols, emitting diagnostic messages). To avoid this problem this PR makes the parsing of OmpArgument directive- sensitive: when the directive is DECLARE_VARIANT, don't attempt to parse a reduction-specifier, consider OmpBaseVariantNames instead. Otherwise ignore OmpBaseVariantNames in favor of reduction-specifier.
Member
|
@llvm/pr-subscribers-flang-parser @llvm/pr-subscribers-flang-semantics Author: Krzysztof Parzyszek (kparzysz) ChangesFull diff: https://github.com/llvm/llvm-project/pull/160595.diff 7 Files Affected:
diff --git a/flang/include/flang/Parser/openmp-utils.h b/flang/include/flang/Parser/openmp-utils.h
index 77c31b939e522..b8f3559097750 100644
--- a/flang/include/flang/Parser/openmp-utils.h
+++ b/flang/include/flang/Parser/openmp-utils.h
@@ -41,7 +41,6 @@ struct ConstructId {
MAKE_CONSTR_ID(OpenMPDeclarativeAllocate, D::OMPD_allocate);
MAKE_CONSTR_ID(OpenMPDeclareReductionConstruct, D::OMPD_declare_reduction);
MAKE_CONSTR_ID(OpenMPExecutableAllocate, D::OMPD_allocate);
-MAKE_CONSTR_ID(OpenMPRequiresConstruct, D::OMPD_requires);
#undef MAKE_CONSTR_ID
@@ -94,8 +93,7 @@ struct DirectiveNameScope {
return std::get<OmpBeginDirective>(x.t).DirName();
} else if constexpr (std::is_same_v<T, OpenMPDeclarativeAllocate> ||
std::is_same_v<T, OpenMPDeclareReductionConstruct> ||
- std::is_same_v<T, OpenMPExecutableAllocate> ||
- std::is_same_v<T, OpenMPRequiresConstruct>) {
+ std::is_same_v<T, OpenMPExecutableAllocate>) {
return MakeName(std::get<Verbatim>(x.t).source, ConstructId<T>::id);
} else {
return GetFromTuple(
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index bd55166eb9f80..8b23189bc1e90 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4991,9 +4991,8 @@ struct OpenMPGroupprivate {
// 2.4 requires -> REQUIRES requires-clause[ [ [,] requires-clause]...]
struct OpenMPRequiresConstruct {
- TUPLE_CLASS_BOILERPLATE(OpenMPRequiresConstruct);
+ WRAPPER_CLASS_BOILERPLATE(OpenMPRequiresConstruct, OmpDirectiveSpecification);
CharBlock source;
- std::tuple<Verbatim, OmpClauseList> t;
};
// 2.15.2 threadprivate -> THREADPRIVATE (variable-name-list)
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 5fd3c1768a17f..ea09fe04f07a0 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1836,7 +1836,9 @@ TYPE_PARSER(sourced( //
// 2.4 Requires construct
TYPE_PARSER(sourced(construct<OpenMPRequiresConstruct>(
- verbatim("REQUIRES"_tok), Parser<OmpClauseList>{})))
+ predicated(OmpDirectiveNameParser{},
+ IsDirective(llvm::omp::Directive::OMPD_requires)) >=
+ Parser<OmpDirectiveSpecification>{})))
// 2.15.2 Threadprivate directive
TYPE_PARSER(sourced( //
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 9812a656092ac..0fbd347e91b18 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2594,10 +2594,10 @@ class UnparseVisitor {
Put("\n");
EndOpenMP();
}
- void Unparse(const OpenMPRequiresConstruct &y) {
+ void Unparse(const OpenMPRequiresConstruct &x) {
BeginOpenMP();
- Word("!$OMP REQUIRES ");
- Walk(std::get<OmpClauseList>(y.t));
+ Word("!$OMP ");
+ Walk(x.v);
Put("\n");
EndOpenMP();
}
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 6538e0b794791..223523f2a46ff 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -624,10 +624,6 @@ template <typename Checker> struct DirectiveSpellingVisitor {
checker_(x.v.DirName().source, Directive::OMPD_groupprivate);
return false;
}
- bool Pre(const parser::OpenMPRequiresConstruct &x) {
- checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_requires);
- return false;
- }
bool Pre(const parser::OmpBeginDirective &x) {
checker_(x.DirName().source, x.DirId());
return false;
@@ -1498,14 +1494,13 @@ void OmpStructureChecker::Leave(const parser::OpenMPDepobjConstruct &x) {
}
void OmpStructureChecker::Enter(const parser::OpenMPRequiresConstruct &x) {
- const auto &dir{std::get<parser::Verbatim>(x.t)};
- PushContextAndClauseSets(dir.source, llvm::omp::Directive::OMPD_requires);
+ const auto &dirName{x.v.DirName()};
+ PushContextAndClauseSets(dirName.source, dirName.v);
if (visitedAtomicSource_.empty()) {
return;
}
- const auto &clauseList{std::get<parser::OmpClauseList>(x.t)};
- for (const parser::OmpClause &clause : clauseList.v) {
+ for (const parser::OmpClause &clause : x.v.Clauses().v) {
llvm::omp::Clause id{clause.Id()};
if (id == llvm::omp::Clause::OMPC_atomic_default_mem_order) {
parser::MessageFormattedText txt(
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 7ef211c8b428c..a4c8922f58c6c 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -523,7 +523,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
// Gather information from the clauses.
Flags flags;
std::optional<common::OmpMemoryOrderType> memOrder;
- for (const auto &clause : std::get<parser::OmpClauseList>(x.t).v) {
+ for (const parser::OmpClause &clause : x.v.Clauses().v) {
flags |= common::visit(
common::visitors{
[&memOrder](
diff --git a/flang/test/Parser/OpenMP/requires.f90 b/flang/test/Parser/OpenMP/requires.f90
new file mode 100644
index 0000000000000..6cbb06eaf93c0
--- /dev/null
+++ b/flang/test/Parser/OpenMP/requires.f90
@@ -0,0 +1,33 @@
+!RUN: %flang_fc1 -fdebug-unparse -fopenmp -fopenmp-version=50 %s | FileCheck --ignore-case --check-prefix="UNPARSE" %s
+!RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=50 %s | FileCheck --check-prefix="PARSE-TREE" %s
+
+!$omp requires atomic_default_mem_order(seq_cst)
+
+!UNPARSE: !$OMP REQUIRES ATOMIC_DEFAULT_MEM_ORDER(SEQ_CST)
+
+!PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPRequiresConstruct -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = requires
+!PARSE-TREE: | OmpClauseList -> OmpClause -> AtomicDefaultMemOrder -> OmpAtomicDefaultMemOrderClause -> OmpMemoryOrderType = Seq_Cst
+!PARSE-TREE: | Flags = None
+
+!$omp requires unified_shared_memory unified_address
+
+!UNPARSE: !$OMP REQUIRES UNIFIED_SHARED_MEMORY UNIFIED_ADDRESS
+
+!PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPRequiresConstruct -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = requires
+!PARSE-TREE: | OmpClauseList -> OmpClause -> UnifiedSharedMemory
+!PARSE-TREE: | OmpClause -> UnifiedAddress
+!PARSE-TREE: | Flags = None
+
+!$omp requires dynamic_allocators reverse_offload
+
+!UNPARSE: !$OMP REQUIRES DYNAMIC_ALLOCATORS REVERSE_OFFLOAD
+
+!PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPRequiresConstruct -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = requires
+!PARSE-TREE: | OmpClauseList -> OmpClause -> DynamicAllocators
+!PARSE-TREE: | OmpClause -> ReverseOffload
+!PARSE-TREE: | Flags = None
+
+end
|
tblah
approved these changes
Sep 24, 2025
Contributor
tblah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Oct 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.